Skip to content

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Mar 20, 2018

Mutable statics don't work in my PR to fix the standalone miri, as init_static didn't get called when the interpret_interner already contained a entry for the static, which is always immutable.

cc rust-lang/miri#364

@rust-highfive
Copy link
Contributor

r? @estebank

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 20, 2018
@estebank
Copy link
Contributor

Test failure:

[00:42:07] ---- [ui] ui/const-eval/issue-47971.rs stdout ----
[00:42:07] 	
[00:42:07] error: test compilation failed although it shouldn't!
[00:42:07] status: exit code: 101
[00:42:07] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/const-eval/issue-47971.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/const-eval/issue-47971.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Zmiri" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/const-eval/issue-47971.stage2-x86_64-unknown-linux-gnu.aux" "-A" "unused"
[00:42:07] stdout:
[00:42:07] ------------------------------------------
[00:42:07] 
[00:42:07] ------------------------------------------
[00:42:07] stderr:
[00:42:07] ------------------------------------------
[00:42:07] {"message":"cyclic dependency detected","code":{"code":"E0391","explanation":"\nThis error indicates that some types or traits depend on each other\nand therefore cannot be constructed.\n\nThe following example contains a circular dependency between two traits:\n\n```compile_fail,E0391\ntrait FirstTrait : SecondTrait {\n\n}\n\ntrait SecondTrait : FirstTrait {\n\n}\n```\n"},"level":"error","spans":[{"file_name":"/checkout/src/test/ui/const-eval/issue-47971.rs","byte_start":614,"byte_end":616,"line_start":19,"line_end":19,"column_start":19,"column_end":21,"is_primary":true,"text":[{"text":"static T: S = S(g(&T), 0);","highlight_start":19,"highlight_end":21}],"label":"cyclic reference","suggested_replacement":null,"expansion":null}],"children":[{"message":"the cycle begins when const-evaluating `T`...","code":null,"level":"note","spans":[{"file_name":"/checkout/src/test/ui/const-eval/issue-47971.rs","byte_start":596,"byte_end":622,"line_start":19,"line_end":19,"column_start":1,"column_end":27,"is_primary":true,"text":[{"text":"static T: S = S(g(&T), 0);","highlight_start":1,"highlight_end":27}],"label":null,"suggested_replacement":null,"expansion":null}],"children":[],"rendered":null},{"message":"...which then again requires const-evaluating `T`, completing the cycle.","code":null,"level":"note","spans":[],"children":[],"rendered":null}],"rendered":"error[E0391]: cyclic dependency detected\n  --> /checkout/src/test/ui/const-eval/issue-47971.rs:19:19\n   |\nLL | static T: S = S(g(&T), 0);\n   |                   ^^ cyclic reference\n   |\nnote: the cycle begins when const-evaluating `T`...\n  --> /checkout/src/test/ui/const-eval/issue-47971.rs:19:1\n   |\nLL | static T: S = S(g(&T), 0);\n   | ^^^^^^^^^^^^^^^^^^^^^^^^^^\n   = note: ...which then again requires const-evaluating `T`, completing the cycle.\n\n"}
[00:42:07] {"message":"aborting due to previous error","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to previous error\n\n"}
[00:42:07] {"message":"For more information about this error, try `rustc --explain E0391`.","code":null,"level":"","spans":[],"children":[],"rendered":"For more information about this error, try `rustc --explain E0391`.\n"}
[00:42:07] 
[00:42:07] ------------------------------------------
[00:42:07] 
[00:42:07] thread '[ui] ui/const-eval/issue-47971.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:2903:9
[00:42:07] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:42:07] 
[00:42:07] 
[00:42:07] failures:
[00:42:07]     [ui] ui/const-eval/issue-47971.rs
[00:42:07] 
[00:42:07] test result: FAILED. 1281 passed; 1 failed; 4 ignored; 0 measured; 0 filtered out
[00:42:07] 

@oli-obk
Copy link
Contributor

oli-obk commented Mar 21, 2018

I think you need to adjust https://github.com/rust-lang/rust/blob/master/src/librustc_mir/interpret/const_eval.rs#L343 to check whether the static has already been computed. If so, don't call tcx const_eval.

@bjorn3
Copy link
Member Author

bjorn3 commented Mar 21, 2018

@oli-obk the problem is that I use a custom init_static to call const_eval and copy it to a mutable alloc which is returned, however the second time the static Place is evaluated eval_place shortcuts and just returns the immutably cached static from const_eval without even calling init_static.

@oli-obk
Copy link
Contributor

oli-obk commented Mar 21, 2018

Oh I get that. I meant rustc's init static impl. The test failure is from that

@bjorn3
Copy link
Member Author

bjorn3 commented Mar 21, 2018

I get it

@bjorn3 bjorn3 force-pushed the patch-1 branch 2 times, most recently from 34ff964 to 5f73901 Compare March 21, 2018 13:40
@bjorn3
Copy link
Member Author

bjorn3 commented Mar 21, 2018

Travis is green

@estebank
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Mar 21, 2018

📌 Commit 5aa29c4 has been approved by estebank

@bors
Copy link
Collaborator

bors commented Mar 21, 2018

🌲 The tree is currently closed for pull requests below priority 30, this pull request will be tested once the tree is reopened

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 21, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Mar 21, 2018
Don't check interpret_interner when accessing a static to fix miri mutable statics

Mutable statics don't work in my PR to fix the standalone [miri](https://github.com/solson/miri), as init_static didn't get called when the interpret_interner already contained a entry for the static, which is always immutable.

cc rust-lang/miri#364
kennytm added a commit to kennytm/rust that referenced this pull request Mar 22, 2018
Don't check interpret_interner when accessing a static to fix miri mutable statics

Mutable statics don't work in my PR to fix the standalone [miri](https://github.com/solson/miri), as init_static didn't get called when the interpret_interner already contained a entry for the static, which is always immutable.

cc rust-lang/miri#364
bors added a commit that referenced this pull request Mar 22, 2018
@alexcrichton alexcrichton merged commit 5aa29c4 into rust-lang:master Mar 22, 2018
@bjorn3 bjorn3 deleted the patch-1 branch March 23, 2018 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants